[input] Added controller support #518

Big-Smarty · 2022-12-29T18:03:20Z

Added controller support.
Abstracted input by adding the Input class, overarching both KeyboardMouseInputData and the also newly added GamepadInputData.

…updating the camera twice per frame.

IAmNotHanni · 2022-12-29T18:11:57Z

Nice work! I think I will buy myself a controller just to check if it's working. Will review later

IceflowRE

👍

IceflowRE · 2022-12-29T18:46:39Z

include/inexor/vulkan-renderer/input/keyboard_mouse_data.hpp

+    [[nodiscard]] glm::ivec2 get_cursor_pos() const;

    /// @brief Calculate the change in x- and y-position of the cursor.
    /// @return a std::array of size 2 which contains the change in x-position in index 0 and the change in y-position
    /// in index 1
-    [[nodiscard]] std::array<double, 2> calculate_cursor_position_delta();
+    [[nodiscard]] glm::dvec2 calculate_cursor_position_delta();


Do we use glm types now everywhere or do we abstract them away to keep independent?
@IAmNotHanni @yeetari

I think using glm here is ok.

IceflowRE · 2022-12-29T18:48:49Z

compile_commands.json

@@ -0,0 +1,212 @@
+[


This file shouldn't be here.

IceflowRE · 2022-12-29T18:49:45Z

include/inexor/vulkan-renderer/input/gamepad_data.hpp

+#include "glm/detail/qualifier.hpp"
+#include <GLFW/glfw3.h>
+#include <vector>
+
+#define GLM_PRECISION_HIGHP_DOUBLE
+#define GLM_PRECISION_HIGHP_INT
+#include <glm/glm.hpp>
+
+#include <array>
+#include <shared_mutex>


Suggested change

#include "glm/detail/qualifier.hpp"

#include <GLFW/glfw3.h>

#include <vector>

#define GLM_PRECISION_HIGHP_DOUBLE

#define GLM_PRECISION_HIGHP_INT

#include <glm/glm.hpp>

#include <array>

#include <shared_mutex>

#include <glm/detail/qualifier.hpp>

#include <GLFW/glfw3.h>

#define GLM_PRECISION_HIGHP_DOUBLE

#define GLM_PRECISION_HIGHP_INT

#include <glm/glm.hpp>

#include <array>

#include <shared_mutex>

#include <vector>

Ah yes: We have rules for ordering include directives.

IceflowRE · 2022-12-29T18:50:12Z

include/inexor/vulkan-renderer/input/input.hpp

+#include <GLFW/glfw3.h>
+
+#include "inexor/vulkan-renderer/input/gamepad_data.hpp"
+#include "inexor/vulkan-renderer/input/keyboard_mouse_data.hpp"


Suggested change

#include <GLFW/glfw3.h>

#include "inexor/vulkan-renderer/input/gamepad_data.hpp"

#include "inexor/vulkan-renderer/input/keyboard_mouse_data.hpp"

#include "inexor/vulkan-renderer/input/gamepad_data.hpp"

#include "inexor/vulkan-renderer/input/keyboard_mouse_data.hpp"

#include <GLFW/glfw3.h>

Same: rules for odering includes

IceflowRE · 2022-12-29T18:50:57Z

src/CMakeLists.txt

    vulkan-renderer/input/keyboard_mouse_data.cpp
+    vulkan-renderer/input/gamepad_data.cpp
+    vulkan-renderer/input/input.cpp


Suggested change

vulkan-renderer/input/keyboard_mouse_data.cpp

vulkan-renderer/input/gamepad_data.cpp

vulkan-renderer/input/input.cpp

vulkan-renderer/input/gamepad_data.cpp

vulkan-renderer/input/input.cpp

vulkan-renderer/input/keyboard_mouse_data.cpp

Yes: keep in alphabetical order

IceflowRE · 2022-12-29T18:52:25Z

src/vulkan-renderer/input/gamepad_data.cpp

+#include <inexor/vulkan-renderer/input/gamepad_data.hpp>
+#include <mutex>


Suggested change

#include <inexor/vulkan-renderer/input/gamepad_data.hpp>

#include <mutex>

#include "inexor/vulkan-renderer/input/gamepad_data.hpp"

#include <mutex>

IceflowRE · 2022-12-29T18:53:06Z

src/vulkan-renderer/input/input.cpp

+#include "GLFW/glfw3.h"
+#include "spdlog/spdlog.h"
+#include <inexor/vulkan-renderer/input/input.hpp>


Suggested change

#include "GLFW/glfw3.h"

#include "spdlog/spdlog.h"

#include <inexor/vulkan-renderer/input/input.hpp>

#include "inexor/vulkan-renderer/input/input.hpp"

#include <GLFW/glfw3.h>

#include <spdlog/spdlog.h>

IceflowRE · 2022-12-29T18:53:56Z

src/vulkan-renderer/input/input.cpp

+#include <inexor/vulkan-renderer/input/input.hpp>
+
+namespace inexor::vulkan_renderer::input {
+void Input::key_callback(GLFWwindow * /*window*/, int key, int, int action, int /*mods*/) {


Suggested change

void Input::key_callback(GLFWwindow * /*window*/, int key, int, int action, int /*mods*/) {

void Input::key_callback(GLFWwindow * /*window*/, int key, int /*scancode*/, int action, int /*mods*/) {

Ah yes: That's the standard way of saying "there once was a parameter" and also to mention the name of the parameter.

IceflowRE · 2022-12-29T18:54:31Z

src/vulkan-renderer/input/keyboard_mouse_data.cpp

 #include "inexor/vulkan-renderer/input/keyboard_mouse_data.hpp"
+#include "GLFW/glfw3.h"
+#include "glm/fwd.hpp"


Suggested change

#include "inexor/vulkan-renderer/input/keyboard_mouse_data.hpp"

#include "GLFW/glfw3.h"

#include "glm/fwd.hpp"

#include "inexor/vulkan-renderer/input/keyboard_mouse_data.hpp"

#include <GLFW/glfw3.h>

#include <glm/fwd.hpp>

IceflowRE · 2022-12-29T18:54:59Z

src/vulkan-renderer/wrapper/window.cpp

 #include "inexor/vulkan-renderer/wrapper/window.hpp"
+#include "GLFW/glfw3.h"

 #include <spdlog/spdlog.h>


Suggested change

#include "inexor/vulkan-renderer/wrapper/window.hpp"

#include "GLFW/glfw3.h"

#include <spdlog/spdlog.h>

#include "inexor/vulkan-renderer/wrapper/window.hpp"

#include <GLFW/glfw3.h>

#include <spdlog/spdlog.h>

IceflowRE · 2022-12-29T18:57:46Z

Other things:
Can you rename the commits that they match our naming convention? See the Github Actions.
Ignore the failed building for now, it is broken, will see when i get time for that.

This will only support one Gamepad right? not multiple?

IAmNotHanni · 2022-12-30T13:23:45Z

@IceflowRE About multiple gamepads/joysticks: I just ordered several online. We should do it in another pull request pls.

IAmNotHanni · 2022-12-30T13:26:53Z

Update docs

https://inexor-vulkan-renderer.readthedocs.io/en/latest/development/reference/keyboard-mouse-input.html
The reference page about input in the docs states:

Inexor does not support joysticks yet.

That's no longer true. Also, the ref pages needs to be renamed to "keyboard, mouse and joystick input"
Furthermore, the docs about keyboard and mouse are now outdated as well.
I will take care of this. I will push into this pull request then.

yeetari

Nice work, code looks good, please could you squash the commits though.

yeetari · 2022-12-30T20:38:45Z

include/inexor/vulkan-renderer/input/gamepad_data.hpp

+#define GLM_PRECISION_HIGHP_DOUBLE
+#define GLM_PRECISION_HIGHP_INT


Suggested change

#define GLM_PRECISION_HIGHP_DOUBLE

#define GLM_PRECISION_HIGHP_INT

These don't do anything.

yeetari · 2022-12-30T20:44:08Z

src/vulkan-renderer/application.cpp


-    if (m_camera->type() == CameraType::LOOK_AT && m_input_data->is_mouse_button_pressed(GLFW_MOUSE_BUTTON_LEFT)) {
+    auto deadzone_lambda = [](const float state) {
+        if (state > -0.2f && state < 0.2f) {


How were these constants chosen? Also it might be easier written as glm::abs(state) < 0.2f (at least for me it looks pretty confusing at first)

Yeah the constants should be named as static constexpr WHAT_IS_MY_NAME{0.2f};

yeetari · 2022-12-30T20:47:35Z

src/vulkan-renderer/application.cpp

-    m_camera->set_movement_state(CameraMovement::LEFT, m_input_data->is_key_pressed(GLFW_KEY_A));
-    m_camera->set_movement_state(CameraMovement::BACKWARD, m_input_data->is_key_pressed(GLFW_KEY_S));
-    m_camera->set_movement_state(CameraMovement::RIGHT, m_input_data->is_key_pressed(GLFW_KEY_D));
+    if (m_camera->type() == CameraType::LOOK_AT) {


This works for now, though in future the Input class should abstract away whether the input event came from kb&m or controller, with a mapping system (so both a controller and mouse could be mapped to a "vertical" and "horizontal" axis that the application can query from m_input).

Yeah in the future I want to refactor the camera code anyways (see #474)

yeetari · 2022-12-30T20:49:06Z

include/inexor/vulkan-renderer/input/gamepad_data.hpp

+private:
+    std::array<glm::vec2, 2> m_current_joystick_axes{};
+    std::array<glm::vec2, 2> m_previous_joystick_axes{};
+    std::array<std::array<bool, GLFW_GAMEPAD_BUTTON_LAST + 1>, GLFW_GAMEPAD_BUTTON_LAST> m_button_states{};


I think the outer array size should be GLFW_JOYSTICK_LAST + 1?

yeetari · 2022-12-30T20:50:53Z

src/vulkan-renderer/input/input.cpp

+}
+
+void Input::update_gamepad_data() {
+    if (glfwJoystickIsGamepad(GLFW_JOYSTICK_1) == 1) {


Could early return here to make it a little easier to read.

Big-Smarty added 3 commits December 29, 2022 18:46

Added controller support and further abstracted the input.

9e9a297

Fixed the issue where only either kbm or controller could be used by …

24adadf

…updating the camera twice per frame.

removed accidentally added clangd cache

7a17f79

IAmNotHanni self-requested a review December 29, 2022 18:10

IAmNotHanni assigned IAmNotHanni and Big-Smarty Dec 29, 2022

IAmNotHanni added the feat:input keybord/mouse input label Dec 29, 2022

IAmNotHanni requested review from IceflowRE and yeetari December 29, 2022 18:12

IAmNotHanni mentioned this pull request Dec 29, 2022

Refactor camera code #474

Open

5 tasks

IceflowRE requested changes Dec 29, 2022

View reviewed changes

yeetari requested changes Dec 30, 2022

View reviewed changes

IAmNotHanni added the cat:enhancement enhancement/requested feature/update of existing features label May 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[input] Added controller support #518

[input] Added controller support #518

Big-Smarty commented Dec 29, 2022

IAmNotHanni commented Dec 29, 2022

IceflowRE left a comment

IceflowRE Dec 29, 2022

IAmNotHanni Jan 1, 2023

IceflowRE Dec 29, 2022

IceflowRE Dec 29, 2022

IAmNotHanni Dec 30, 2022 •

edited

Loading

IceflowRE Dec 29, 2022

IAmNotHanni Dec 30, 2022

IceflowRE Dec 29, 2022

IAmNotHanni Dec 30, 2022

IceflowRE Dec 29, 2022

IceflowRE Dec 29, 2022

IceflowRE Dec 29, 2022

IAmNotHanni Dec 30, 2022

IceflowRE Dec 29, 2022

IceflowRE Dec 29, 2022

IceflowRE commented Dec 29, 2022

IAmNotHanni commented Dec 30, 2022

IAmNotHanni commented Dec 30, 2022 •

edited

Loading

yeetari left a comment

yeetari Dec 30, 2022

yeetari Dec 30, 2022

IAmNotHanni Jan 1, 2023

yeetari Dec 30, 2022

IAmNotHanni Dec 30, 2022

yeetari Dec 30, 2022

yeetari Dec 30, 2022

		#include <inexor/vulkan-renderer/input/gamepad_data.hpp>
		#include <mutex>

	void Input::key_callback(GLFWwindow * /window/, int key, int, int action, int /mods/) {
	void Input::key_callback(GLFWwindow * /window/, int key, int /scancode/, int action, int /mods/) {

		#define GLM_PRECISION_HIGHP_DOUBLE
		#define GLM_PRECISION_HIGHP_INT

[input] Added controller support #518

Are you sure you want to change the base?

[input] Added controller support #518

Conversation

Big-Smarty commented Dec 29, 2022

IAmNotHanni commented Dec 29, 2022

IceflowRE left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IAmNotHanni Dec 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IceflowRE commented Dec 29, 2022

IAmNotHanni commented Dec 30, 2022

IAmNotHanni commented Dec 30, 2022 • edited Loading

yeetari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IAmNotHanni Dec 30, 2022 •

edited

Loading

IAmNotHanni commented Dec 30, 2022 •

edited

Loading